[IR][Instructions] Add CallBase::getCalledFunctionName helper
#127038
Conversation
There's a lot of uses of `getCalledFunction()->getName()`, without ever checking if `getCalledFunction()` returns a nullptr. Add a new helper that returns an `std::optional<StringRef>`, that can be used to - Avoid method chaining - Have a more safe shortcut to get the callee name.
|
@llvm/pr-subscribers-flang-openmp @llvm/pr-subscribers-llvm-ir Author: Thomas Symalla (tsymalla) ChangesThere's a lot of uses of
Full diff: https://github.com/llvm/llvm-project/pull/127038.diff 1 Files Affected:
diff --git a/llvm/include/llvm/IR/InstrTypes.h b/llvm/include/llvm/IR/InstrTypes.h
index 90fe864d4ae71..c55a5a589fc53 100644
--- a/llvm/include/llvm/IR/InstrTypes.h
+++ b/llvm/include/llvm/IR/InstrTypes.h
@@ -1345,6 +1345,16 @@ class CallBase : public Instruction {
return nullptr;
}
+ /// Shortcut to retrieving the name of the called function.
+ /// Returns std::nullopt, if the function cannot be found.
+ std::optional<StringRef> getCalledFunctionName() const {
+ Function *F = getCalledFunction();
+ if (F)
+ return F->getName();
+
+ return std::nullopt;
+ }
+
/// Return true if the callsite is an indirect call.
bool isIndirectCall() const;
|
nikic
left a comment
nikic
left a comment
There was a problem hiding this comment.
If you're introducing a new helper API, please also update existing code that can make use of it.
| if (F) | ||
| return F->getName(); | ||
|
|
||
| return std::nullopt; |
There was a problem hiding this comment.
Can we use an empty StringRef as the sentinel instead?
There was a problem hiding this comment.
Technically, yes, but I think std::optional is a better fit, since using operator* on the return value would fail if no value is present.
There was a problem hiding this comment.
Which is the kind of failure you are trying to avoid needing to worry about?
antoniofrighetto
changed the title
CallBase::getCalledFunctionName helper
`LibCallsShrinkWrap::perform`: This uses the potentially `nullptr` value inside the loop in a debug statement before asserting on it. Move the assertion outside `perform(CallInst &)`. Some other arbitrary uses to replace `getCalledFunction()->getName()`.
llvmbot
added
llvm:analysis
nikic
left a comment
nikic
left a comment
There was a problem hiding this comment.
From a quick git grep "getCalledFunction()->getName()", it looks like there are a lot more places that could use this.
Yep. I wanted to see if the change is generally accepted at first, before changing all the code. I'll follow up with the remainder. |
llvmbot
added
backend:AArch64
backend:AMDGPU
vectorizers
PGO
|
✅ With the latest revision this PR passed the C/C++ code formatter. |
|
Sorry for the massive update. All callsites of |
5 participants
There's a lot of uses of
getCalledFunction()->getName(), without ever checking ifgetCalledFunction()returns a nullptr. Add a new helper that returns anstd::optional<StringRef>, that can be used to